Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

InlineArray design spec/doc #108044

Merged
merged 14 commits into from
Sep 25, 2024
Merged

InlineArray design spec/doc #108044

merged 14 commits into from
Sep 25, 2024

Conversation

VSadov
Copy link
Member

@VSadov VSadov commented Sep 20, 2024

Fixes: #91012

  • Convert the proposal into a document and move to appropriate place.
  • Add some info about layout of the inline array instance (i.e. pointer to the entire struct and to the first element are equal,...)
  • Add a note on readonly element. This has no special semantics and as such unsupported, not recommended.
  • Add a note on the max size of an instance.

@VSadov VSadov added the documentation Documentation bug or enhancement, does not impact product or test code label Sep 20, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Sep 20, 2024
@VSadov
Copy link
Member Author

VSadov commented Sep 20, 2024

CC: @mangod9 @jkotas @MichalStrehovsky @lambdageek - since you've had interest or contributed to this in the past

There were multiple proposals and ideas in the past asking for a no-indirection primitive for inline data. [Example1(inline strings)](https://github.com/dotnet/csharplang/issues/2099), [Example2(inline arrays)](https://github.com/dotnet/runtime/issues/12320), [Example3(generalized fixed-buffers)](https://github.com/dotnet/csharplang/blob/main/proposals/low-level-struct-improvements.md#safe-fixed-size-buffers)
Our preexisting offering in this area – [unsafe fixed-sized buffers](https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/unsafe-code#fixed-size-buffers) has multiple constraints, in particular it works only with blittable value types and provides no overrun/type safety, which considerably limits its use.

*The InlineArrayAttribute is a building block to allow efficient, type-safe, overrun-safe indexable/sliceable inline data.*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The InlineArrayAttribute alone does not address overrun-safety.

Related to #105586.

Copy link
Member Author

@VSadov VSadov Sep 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By itself InlineArray provides only storage. Thus I call it a "building block", not "a type-safe, overrun-safe indexable data". It is not that.

Since the type does not know its length or how to index itself, the indexing and range checking is the responsibility of the higher level implementation (hand-made or compiler-generated) that wraps the storage.

In a way the C#-provided indexing is still safe as one cannot do arr[5] on an array with [InlineArray(length:3)], it is just enforced by the layer that is above IL/typesystem.

This is one tradeoff that we took when we did not go the InlineArray<T, Length> path.
That would have indexing and range checking at the runtime level - without wrappers/helpers.

I think "building block" is vague enough to hint at the purpose of the feature.
We do not need to prescribe here how one can actually build indexable things with this, but including a link to C# feature design/spec would be a good example.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

including a link to C# feature design/spec would be a good example.

Added.

@jkotas
Copy link
Member

jkotas commented Sep 20, 2024

Include a link to the matching Roslyn proposal?

@am11 am11 added area-System.Runtime.CompilerServices and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Sep 21, 2024

`Length` must be greater than 0.

struct must not have explicit layout.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the behavior if the struct has Auto layout? Is it guaranteed the same as sequential and that it will be exactly sizeof(T) * Length (where T is the type of the singular instance field) long with all data starting at offset 0?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be the same for Auto layout. Since we require exactly one instance field, there is not much difference between Sequential and Auto for the first element.
If there are more elements, they start at sizeof(T) * n offsets.

In particular (using the `MyArray<T>` example defined above):
* In unboxed form there is no object header or any other data before the first element.

Example: assuming the instance is not GC-movable, the following holds: `(byte*)*inst == (byte*)inst._element0`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expression doesn't look like a valid C#. *inst is dereferencing something that is not a pointer, inst._element0 is not a pointer.


* There is no additional padding between elements.

Example: assuming the instance is not GC-movable and `Length > 1`, the following will yield a pointer to the second element: `(byte*)inst._element0 + sizeof(T)`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expression doesn't look like a valid C#.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. Not sure why & are missing.
I think I initially wanted to make examples in terms of ref, but since we do not formally allow math operations with ref, I'd need to use Unsafe and it felt like not helping to understand the layout invariants.

@AlekseyTs
Copy link
Contributor

It is probably worth to specify behavior of Equals/GetHashCode for inline array structures

@VSadov
Copy link
Member Author

VSadov commented Sep 24, 2024

Any more comments on this?

@VSadov
Copy link
Member Author

VSadov commented Sep 25, 2024

I think all actionable comments have been addressed, so I will merge it.

Design documents are live documents though, so if something comes up, please let me know. We can patch this up.

@VSadov VSadov merged commit 782c2ae into dotnet:main Sep 25, 2024
13 checks passed
@VSadov VSadov deleted the VSadov-patch-1 branch September 25, 2024 03:44
sirntar pushed a commit to sirntar/runtime that referenced this pull request Sep 30, 2024
* Create InlineArrayAttribute.md

* Update InlineArrayAttribute.md

* about readonly field

* On type layout

* Update InlineArrayAttribute.md

* On max size

* make linter happy

* more linter failures

* linter failure (hopefully the last one)

* small wording tweak

* Apply suggestions from code review

Co-authored-by: Jan Kotas <[email protected]>

* Added a link to C# Inline Arrays, as an example where this is used.

* Update InlineArrayAttribute.md

* Default behavior of of Equals() and GetHashCode()

---------

Co-authored-by: Jan Kotas <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Oct 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime.CompilerServices documentation Documentation bug or enhancement, does not impact product or test code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

InlineArray documentation tracking
6 participants